Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(configio): config encoding #66

Draft
wants to merge 53 commits into
base: main
Choose a base branch
from
Draft

Conversation

GregoryAlbouy
Copy link
Member

⚠️ DO NOT MERGE until it targets master

Description

Changes

Notes

- ParseRepresentation -> UnmarshalRepresentation: change signature
so it matches json.Unmarshal style
- implement Representation.Unmarshal: alias to UnmarshalRepresentation
- update JSON accordingly
- remove Config.Override, Config.WithFields,
  Config.Equal
- remove related tests
- update tests relying on removed Config methods
- module github.com/benchttp/engine -> github.com/benchttp/sdk
- package runner -> benchttp
Irrelevant method, use Runner.OnProgress instead.
- simplify with less declarations
- move into fie runner.go
- more accurate docs
Allows convenient chaining, e.g. when using de default runner:
DefaultRunner().WithNewRequest("", "http://a.b", nil).Run(ctx)

- Runner.WithRequest
- Runner.WithNewRequest
- implement AssertEqualRunners, EqualRunners, DiffRunner
- Unit test exposed functions
Rethink exposed API so it matches the native json lib.

- replace Parse -> Decode in all declarations and file names
- expose top-level unmarshaling & decoding functions:
  - UnmarshalJSON, UnmarshalYAML: unmarshal bytes into *Representation
  - UnmarshalJSONRunner, UnmarshalYAMLRunner: same into *benchttp.Runner
  - NewJSONDecoder, NewYAMLDecoder: same as json.NewDecoder
  - DecoderOf: returns appropriate Decoder given an extension
- make testdata more robust easy to reason about using a dedicated
  package
- use table testing
- use benchttptest for runner comparison
- extract assertXxx helpers
- use dedicated data structure (file)
- rethink namings
- move functions to appropriate places
Previous benchmarks showed 2.5x better performance over piped modifier.
- Builder.Into -> Builder.Mutate
- Builder.modifiers -> Builder.mutations
- configio.Representation -> configio.representation
- make all representation methods private
- remove Decode methods from interface Decoder and its implementations
- rename DecodeRunner methods to Decode
- remove UnmarshalXXXX functions
- rename UnmarshalXXXXRunner functions to UnmarshalXXXX
- configio.representation -> configio/internal/Repr
- expose necessary methods
@GregoryAlbouy GregoryAlbouy added this to the v0.2 milestone Nov 7, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #66 (fe16752) into feat/configio-builder (e865353) will decrease coverage by 0.13%.
The diff coverage is 71.75%.

@@                    Coverage Diff                    @@
##           feat/configio-builder      #66      +/-   ##
=========================================================
- Coverage                  81.64%   81.50%   -0.14%     
=========================================================
  Files                         29       31       +2     
  Lines                       1024     1065      +41     
=========================================================
+ Hits                         836      868      +32     
- Misses                       150      158       +8     
- Partials                      38       39       +1     
Flag Coverage Δ
unittests 81.50% <71.75%> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
configio/decoder.go 63.63% <ø> (ø)
configio/builder.go 82.71% <33.33%> (ø)
configio/internal/conversion/representation.go 38.09% <38.09%> (ø)
configio/internal/conversion/parsing.go 66.17% <66.17%> (ø)
configio/internal/conversion/converters.go 84.21% <84.21%> (ø)
configio/file.go 92.59% <100.00%> (ø)
configio/json.go 94.44% <100.00%> (ø)
configio/yaml.go 91.48% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

todo:
- configio: decoder API
- conversion: decode: fix nil pointer derefence
- conversion: separate encode/decode
  (use dedicated pointerless structure for encode?)
- body: to encode or not to encode (alters request)
- yaml encode
- various refactoring
@GregoryAlbouy GregoryAlbouy force-pushed the feat/configio-builder branch from e865353 to 39fb4a6 Compare March 28, 2023 21:34
Base automatically changed from feat/configio-builder to main March 28, 2023 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants